Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

factor in font ligatures while rendering hints #162

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

purajit
Copy link

@purajit purajit commented Sep 14, 2024

The issue is described in #158 #161.

The problem is the match.x position is essentially taken as the cursor position to display
hints (with some utf-8 adjustments). This works in all cases except when ligatures are
involved, since that's a choice of the font. Ligatures are becoming more and more common -
especially in people's shell prompts, as well as various modern TUIs and CLIs that are meant
to only be consumed by humans and not programs.

We can only know whether we'll have ligatures by asking a font to shape the text. This PR
uses rustybuzz for that.

Alternatives

We could try to do something a bit "clever" (read: hacky): if the string at the cursor
position doesn't match, we can go backwards on the line to find the correct match. This
should be much faster than shaping the font, and potentially more reliable, since we'd be
able to handle any ligatures. Even though it isn't a "precise" solution and just a heuristic,
analyzing and shaping fonts is a pretty chaotic, messy world anyway.

We could change fundamentally how it searches a line by having it be based on cursor advances.
Unsure what kind of impacts this could have on bugs on performance, but I'm definitely not feeling
great about doing this.

Performance

rustybuzz themselves note:

At the moment, performance isn't that great. We're 1.5-2x slower than harfbuzz.

It also looks like they may potentially not include the known optimizations since it makes
the code harder to sync with the harfbuzz, which this is a port of

The end-user experience is still pretty solid - there's maybe a millisecond pause that's
otherwise not there. Considering we're only rendering one screen's worth of text, it doesn't
come into a play too much. I say this as a person that's already used to doing the motion
pretty quickly. I can run some benchmarking if we're serious about this change.

We could potentially use harfbuzz directly and use it as an executable. It's very active
and well-maintained, and much more performant. However, it adds external dependencies.

Notes

The user has to provide font-file as an config. If not provided, we don't bother with
ligatures. If they use different font families that have different ligature behaviors in
different parts of their terminal, there isn't really any way to handle that.

I just currently dumped in the font I'm using for the test; if we want to actually upstream
this PR, I'll create a separate test font with just the minimal components

This PR is kind of a proof-of-concept, but I've written it to be as upstreamable as possible.

There are a couple edge cases to handle; I'll go through those more rigorously if there's
chance of upstreaming.

@purajit purajit marked this pull request as ready for review September 14, 2024 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant